Skip to content

DX-118395: Make SearchTableAndViews resilient to broken catalog entries#103

Merged
ssaumitra merged 9 commits into
dremio:mainfrom
ssaumitra:DX-118395
Apr 27, 2026
Merged

DX-118395: Make SearchTableAndViews resilient to broken catalog entries#103
ssaumitra merged 9 commits into
dremio:mainfrom
ssaumitra:DX-118395

Conversation

@ssaumitra

@ssaumitra ssaumitra commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

A single unresolvable view in the search results (e.g. a view with a disconnected source) caused the catalog-by-path lookup to return HTTP 400, which propagated through asyncio.gather and failed the entire SearchTableAndViews tool call.

In query engine catalogs, it's a common behavior not to delete VDS when dependent PDS is deleted. However, when the schema of such VDS is queried, catalog throws an error. In this fix, we are choosing to be defensive when catalog does not return schema of certain dataset.

get_schemas now isolates per-path failures and returns the error message alongside each schema. SearchTableAndViews surfaces the affected paths in a skipped field plus a human-readable skipped_note, so the user sees the healthy results and a clear explanation for the ones that were dropped.

Also adds CLAUDE.md with project context and development guidelines to help Claude Code sessions work more effectively in this repo.

A single unresolvable view in the search results (e.g. a view with a
disconnected source) caused the catalog-by-path lookup to return HTTP 400,
which propagated through asyncio.gather and failed the entire
SearchTableAndViews tool call.

get_schemas now isolates per-path failures and returns the error message
alongside each schema. SearchTableAndViews surfaces the affected paths in
a `skipped` field plus a human-readable `skipped_note`, so the user sees
the healthy results and a clear explanation for the ones that were
dropped.

Also adds CLAUDE.md with project context and development guidelines to
help Claude Code sessions work more effectively in this repo.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread src/dremioai/api/dremio/catalog.py Outdated
maxlepikhin
maxlepikhin previously approved these changes Apr 22, 2026

@aniket-s-kulkarni aniket-s-kulkarni left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Verdict: Must-fix design gaps — 3 must fix, 1 should fix

Must fix:

  • get_schemas return type should be a Pydantic BaseModel (e.g. SchemaResult) not a raw tuple — src/dremioai/api/dremio/catalog.py
  • get_descriptions now silently swallows catalog errors, changing its original fail-fast contract — src/dremioai/api/dremio/catalog.py
  • Missing server-side logging when a search hit's schema fetch fails — src/dremioai/api/dremio/search.py

Should fix:

  • get_lineage still calls get_schema (singular) and accesses response["id"] directly — coordinate now to avoid a future silent break if get_schema is later changed to return SchemaResultsrc/dremioai/api/dremio/catalog.py:119

AI-assisted analysis

Comment thread src/dremioai/api/dremio/catalog.py Outdated
Comment thread src/dremioai/api/dremio/catalog.py Outdated
Comment thread src/dremioai/api/dremio/search.py Outdated
- Replace the untyped `Tuple[Dict, Optional[str]]` return of
  `get_schemas` with a `SchemaResult` Pydantic model (`data`, `error`).
  Gives callers named fields and a shape that can be extended without
  another signature change.
- Fix `get_descriptions`: the original silent-drop-on-success was the
  actual root cause of DX-118395 (a broken VDS whose parent PDS was
  deleted tanked the entire call via asyncio.gather). It now tolerates
  per-path failures, logs each via structlog, and continues with the
  paths that resolved.
- Log skipped paths in `get_search_results` via structlog
  (`search_schema_fetch_failed`) before recording them in `skipped`, so
  operators see the failures in logs during an incident.
- Clean up imports: move `logger = log.logger(__name__)` below all
  `from ... import` lines in both `catalog.py` and `search.py`, and
  drop the now-unused `arbitrary_types_allowed` from `SchemaResult`.
- Update tests to the new `SchemaResult` attribute access.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread src/dremioai/api/dremio/catalog.py Outdated
Comment thread src/dremioai/api/dremio/search.py Outdated
Comment thread src/dremioai/tools/tools.py Outdated
@ssaumitra

Copy link
Copy Markdown
Contributor Author

@aniket-s-kulkarni I have incorporated all of your review responses. Can you please review again?

Comment thread src/dremioai/api/dremio/catalog.py Outdated
Comment thread src/dremioai/api/dremio/search.py Outdated
Comment thread src/dremioai/api/dremio/search.py Outdated
Comment thread src/dremioai/api/dremio/search.py Outdated
Comment thread src/dremioai/api/dremio/search.py Outdated
Comment thread src/dremioai/api/dremio/search.py Outdated
Comment thread src/dremioai/api/dremio/search.py
@ssaumitra ssaumitra merged commit 613f940 into dremio:main Apr 27, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants